Port securesystemslib.hash module by lukpueh · Pull Request #2815 · theupdateframework/python-tuf

lukpueh · 2025-03-18T13:59:59Z

securesystemslib.hash is a small wrapper around hashlib, which serves two main purposes:

provide helper function to hash a file
translate custom hash algorithm name "blake2b-256" to "blake2b" with (digest_size=32).

In preparation for the removal of securesystemslib.hash, this patch ports above behavior to tuf and uses the builtin hashlib directly where possible.

related secure-systems-lab/securesystemslib#943

lukpueh · 2025-03-18T15:01:15Z

tuf/api/_payload.py

 T = TypeVar("T", "Root", "Timestamp", "Snapshot", "Targets")


+def _hash(algo: str) -> Any:  # noqa: ANN401


Python's hashlib does not make the Hash type public, hence I'm using Any and ask the linter for forgiveness.

tuf/api/_payload.py

lukpueh · 2025-03-18T15:08:14Z

tests/repository_simulator.py


 SPEC_VER = ".".join(SPECIFICATION_VERSION)

+_DEFAULT_HASH_ALGORITHM = "sha256"


IMO it's okay to make this a local default, and not re-use the one newly added to _payload.py, so that we can keep the latter internal.

sure, I'd be fine with just a magic value as well: it's not really even a default, it's just what this repository decided to use

securesystemslib.hash is a small wrapper around hashlib, which serves two main purposes: * provide helper function to hash a file * translate custom hash algorithm name "blake2b-256" to "blake2b" with (digest_size=32). In preparation for the removal of securesystemslib.hash, this patch ports above behavior to tuf and uses the builtin hashlib directly where possible. related secure-systems-lab/securesystemslib#943 Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

lukpueh · 2025-03-18T15:42:09Z

tuf/api/_payload.py

+    """Returns hashed file."""
+    f.seek(0)
+    if sys.version_info >= (3, 11):
+        digest = hashlib.file_digest(f, lambda: _hash(algo))  # type: ignore[arg-type]


Mypy does not allow IO[bytes] for f here. This may be related to python/typing#659. Ignore seems fair.

jku

looks fine to me. Left some comments

jku · 2025-03-18T16:05:16Z

tuf/api/_payload.py

+                    digest_object = _hash(algorithm)
                    digest_object.update(data)
                else:
-                    digest_object = sslib_hash.digest_fileobject(
-                        data, algorithm
-                    )
-            except (
-                sslib_exceptions.UnsupportedAlgorithmError,
-                sslib_exceptions.FormatError,
-            ) as e:
+                    digest_object = _file_hash(data, algorithm)
+            except (ValueError, TypeError) as e:


the different functionality is a little annoying here (you need to call update on one digest_object but not the other)...

If the _hash and _file_hash functions directly returned the hexdigest this code would be simpler and the functions would be properly annotated

on the other hand then _hash() would need a data argument (meaning you would have to split the lambda needed by hashlib.file_digest() into another function)

I don't really know what is the simplest... up to you

Does 535a189 look better? I don't have very strong feelings about either way.

No passionate argument here either. I think I like this new version better.

jku · 2025-03-18T16:08:05Z

tests/repository_simulator.py


 SPEC_VER = ".".join(SPECIFICATION_VERSION)

+_DEFAULT_HASH_ALGORITHM = "sha256"


sure, I'd be fine with just a magic value as well: it's not really even a default, it's just what this repository decided to use

tuf/api/_payload.py

coveralls · 2025-03-18T16:45:43Z

coverage: 97.064% (-0.01%) from 97.076%
when pulling 75e83b3 on lukpueh:port-sslib-hash
into 9f873cb on theupdateframework:develop.

jku · 2025-03-19T08:00:19Z

coverage: 96.981% (-0.1%) from 97.076%

coveralls.io is down again so I'm not sure... but I'm guessing this is the "blake2b-256" special case -- otherwise we're probably exercising this code.

Remove the "default" prefix, because it's not a default but rather a fixed value. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

Consolidate interface of bytes hash and file hash helpers. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

lukpueh · 2025-03-19T09:14:41Z

coverage: 96.981% (-0.1%) from 97.076%

coveralls.io is down again so I'm not sure... but I'm guessing this is the "blake2b-256" special case -- otherwise we're probably exercising this code.

Yep, blake is the culprit. Let me add a test.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

lukpueh · 2025-03-19T09:58:39Z

@jku, want to re-approve after my latest changes?

jku

Looks good. The only nit I have left is that multiple docstrings still refer to "securesystemslib default hash algorithm".

Default hash sha256 is now defined locally. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

lukpueh · 2025-03-19T10:30:36Z

Looks good. The only nit I have left is that multiple docstrings still refer to "securesystemslib default hash algorithm".

Good eyes! I pushed a fix and will merge when ci returns.

fixes secure-systems-lab#943 * Internal use does not need the additional features (custom blake algorithm name support and file hashing), and was replaced by direct calls to hashlib. * External users were updated to no longer require `securesystemslib.hash` (theupdateframework/python-tuf#2815, in-toto/in-toto#861) Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

fixes secure-systems-lab#943 * Internally, the features added by the module (i.e. custom blake name support, and file hashing) are not needed. Builtin hashlib is used now directly. * External users port the relevant features into their code base: theupdateframework/python-tuf#2815 in-toto/in-toto#861 Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

lukpueh requested a review from a team as a code owner March 18, 2025 13:59

lukpueh requested review from jku and removed request for jku March 18, 2025 14:00

lukpueh marked this pull request as draft March 18, 2025 14:09

lukpueh force-pushed the port-sslib-hash branch from 369642b to 7ac9f23 Compare March 18, 2025 14:57

lukpueh commented Mar 18, 2025

View reviewed changes

lukpueh force-pushed the port-sslib-hash branch from 7ac9f23 to 866409f Compare March 18, 2025 15:39

lukpueh commented Mar 18, 2025

View reviewed changes

lukpueh marked this pull request as ready for review March 18, 2025 15:42

jku approved these changes Mar 18, 2025

View reviewed changes

Lukas Puehringer added 2 commits March 19, 2025 09:28

Rename hash algo global in repo simulator

57010fb

Remove the "default" prefix, because it's not a default but rather a fixed value. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

Refactor hash helpers

535a189

Consolidate interface of bytes hash and file hash helpers. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

Add tests for custom blake hash

6f50998

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

jku approved these changes Mar 19, 2025

View reviewed changes

docs: Remove reference to securesystemslib hash

75e83b3

Default hash sha256 is now defined locally. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>

lukpueh merged commit 500e8b9 into theupdateframework:develop Mar 19, 2025
17 checks passed

jku mentioned this pull request Mar 19, 2025

Annotate the library (add py.typed) secure-systems-lab/securesystemslib#970

Merged

lukpueh mentioned this pull request Mar 19, 2025

Remove internal use of securesystemslib.hash secure-systems-lab/securesystemslib#977

Merged

		T = TypeVar("T", "Root", "Timestamp", "Snapshot", "Targets")


		def _hash(algo: str) -> Any: # noqa: ANN401


		SPEC_VER = ".".join(SPECIFICATION_VERSION)

		_DEFAULT_HASH_ALGORITHM = "sha256"

Conversation

lukpueh commented Mar 18, 2025

Uh oh!

lukpueh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukpueh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

jku Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

lukpueh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

jku left a comment

Choose a reason for hiding this comment

Uh oh!

jku Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukpueh Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

jku Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

jku Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jku commented Mar 19, 2025

Uh oh!

lukpueh commented Mar 19, 2025

Uh oh!

lukpueh commented Mar 19, 2025

Uh oh!

jku left a comment

Choose a reason for hiding this comment

Uh oh!

lukpueh commented Mar 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jku Mar 18, 2025 •

edited

Loading

coveralls commented Mar 18, 2025 •

edited

Loading